Loading land data later doesn't play nicely with the editor at the moment. The land load code may need some significant refactoring.

My current list of what remains to be done:

1. Figure out the best way to solve the problem with land data delayed loading.
2. Connect and handle signals to update the terrain in the scene view.
3. Import land textures when cloning lands.
4. Add the ability to merge multiple land textures into one.

I'll be taking a break from this for a few days.

I'm getting some strange artifacts with the terrain system. I think it may have to do with colors.

Seems to only be an issue with cloned/touched lands which both copy the data.

Turned out to be an array bounds issue.

Land texture merging can be implemented later with the automatic land texture deletion feature, since both will require indexing the textures used in each land texture.

Just a heads up, I'm likely to lose power/internet access at some point tomorrow, and it may take a few days to come back up, so I may not respond to feedback in a reasonable amount of time. Also, I'll admit I was rushing this toward the end, so I would suggest taking time with the reviewing process, especially in loadland.cpp and columnimp.cpp. I'll review it myself in a day or two assuming I'm able to.

No hurry. Extremely busy right now, so I won't get around to do the review for a couple of days. Entirely possible that you will beat me to it with your own final review.

I'm so glad you have taken this task on. I'll review this as well in the coming days.

Alright, take your time.

Here is my first review pass. Besides a couple of minor issues that I commented on inline I see one major problem. In the LandTexture table there are multiple records with the same texture index (sometimes even more than 2). That is confusing to the user and is not useful. For all intents and purposes LTEX records that are not part of the currently edited plugin don't exist.
Furthermore when I touched a Land record from base, the resulting LandTexture records had very high texture indices (4 digits or so, while everything else seems to be barely above 100. That looks odd and may require further investigation.

In the LandTexture table there are multiple records with the same texture index (sometimes even more than 2). That is confusing to the user and is not useful.

Should we just not show Base LandTextures then?

Furthermore when I touched a Land record from base, the resulting LandTexture records had very high texture indices (4 digits or so, while everything else seems to be barely above 100.

When importing textures that conflict with existing indices (and have a different texture or texture handle/name), a new index is chosen. I spread out these indices because it should reduce the number of future conflicts (when selecting new indices) and I saw no downsides.

Should we just not show Base LandTextures then?

That sounds reasonable.

When importing textures that conflict with existing indices (and have a different texture or texture handle/name), a new index is chosen. I spread out these indices because it should reduce the number of future conflicts (when selecting new indices) and I saw no downsides.

Fair enough. But have you considered the case of multiple editing operations that require creating new LTEX indices. If this is done again and again over multiple content files, would this trick still help with avoiding conflicts and are we at risk of running out of indices eventually.

Not sure if their is a good way to hide the base records. I have made it so they appear grayed out for now.

For containers, I've chose to use a QVector over an std::vector because the QVector should work better with how QVariants work (lots of copies). It would not be difficult to change though.

The performance using a map for texture inputs was decent. On my laptop with an i5-5200u processor it takes 1-2 seconds to touch all land records with Morrowind, Tribunal, and Bloodmoon loaded.

Should the TextureHandle column (which I renamed to TextureNickname) be removed or kept?

Are the textures always case insensitive regardless of the filesystem? I noticed that there are some duplicate textures as a result of having a different case, and if that never matters then we should ignore the case. If it does matter, then there is an issue with the auto completion which seems to always suggests lower case names.

Everything in Morrowind is case-insensitive (stupid Windows!). If OpenMW treats anything non case-sensitive then that is a bug.

wait, what? Shouldn't that read:
If OpenMW treats anything case-sensitive then that is a bug.

Everything should be case-insensitive, thanks to being handicapped by Windows. Right?

Oops. Yes, you are right. The last sentence was incorrect.

I'm getting some errors in the console, but I think they are the result of recent changes and not mine.

Error: RigGeometry rendering with no skeleton, should have been initialized by UpdateVisitor
before Glyph::subload(): detected OpenGL error: invalid value
Warning: detected OpenGL error 'invalid value' at after RenderBin::draw(..)

I get the same errors on the master branch. Regarding the hiding of base LandTexture records, perhaps that could done using the existing search/sort mechanisms?

Regarding the hiding of base LandTexture records, perhaps that could done using the existing search/sort mechanisms?

Don't see how that works since what is displayed in the table is not determined by search/sort. User a filter may be an option if it doesn't interfere with other filtering functions. The only other option I see is to manually change the mapping between records in the collections and rows in the table model.

The only other option I see is to manually change the mapping between records in the collections and rows in the table model.

I don't think we want to do that since we only want to hide the records from the user, and the tables can be accessed for other uses. I'll see if I can figure out a way to make filtering work.

I think everything has been addressed now.

I'll merge that for now. I don't see anything wrong with it, but we definitely need more testing.

I think we still need to think over the "texture nickname" column. It does not make any sense to me. And AFAIK this field isn't used anywhere. Maybe we should just hide it? Or maybe get some input from other modders on the forum first?

With this done the next task will be terrain texture selection. @PlutonicOverkill has done some work on it but I don't know how complete it is. @Aesylwinn Do you want to continue to work on the terrain feature? Unless PlutonicOverkill wants to go on with it, maybe you could take that over his work (#1414).

\o/

This is great news for the next release!

I think we'll need some tutorials for how to use this. :) Or in the next video, that this is demoed!

I've gone ahead and posted a thread on the forums asking for opinions about the unused field. I'm somewhat busy at the moment, so I probably wouldn't be able to continue with the LAND/LTEX related features for at least a month, if PlutonicOverkill does not wish to continue with it.

@psi29a I'm afraid this was more grounds work for terrain editing, so there isn't much you can/should do with it. Touching land records and changing the textures referred to should not be preferred over creating a texture with the same name for mod compatibility reasons.

Hi, I am moving house next week, and since this is a very busy time of year, I can't do any work on the terrain selection feature during the next few weeks. I've got OSG built, but still haven't managed to get everything correctly linked and running, and until that happens I won't be able to continue with it. I also have to admit, I get a bit lost whenever a discussion comes up surrounding LAND and LTEX records, so I'll need to research those properly as well.

The files components/esm/loadland.hpp and components/esm/loadltex.hpp are good places to start. I believe there is a small discussion about LTEX records in the forum post linked in the opening post. You probably also want to look at apps/opencs/model/world/columnimp.hpp for the way to access and set the relevant data. Looking at how this PR "imports" LTEX records would probably also be beneficial. (files are off the top of my head, hopefully they are correct).

Seeing how PlutonicOverkill wants to continue, I'll start working on some of the recent issues that have popped up once I have some time available.